Skip to content

MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test#1691

Open
sandboxcoder wants to merge 2 commits into
stagingfrom
rno/get-properties
Open

MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test#1691
sandboxcoder wants to merge 2 commits into
stagingfrom
rno/get-properties

Conversation

@sandboxcoder
Copy link
Copy Markdown
Contributor

@sandboxcoder sandboxcoder commented May 6, 2026

Description

  • Restore commit 9279b8a now that this has been fully root caused. Bringing back this change because it fixes the case where coreaudio spins up a reconnect_thread whenever Desktop queries for audio devices. This was because the previous implementation passed in a dummy device which causes coreaudio to create a thread to poll if this dummy device is later initialized. This will cleanup the logs eliminating this warning (32 occurrences in a log I checked from Desktop): [Warning] [coreaudio_get_device_name] failed to get name: 560947818
  • Set CI env flag for mac test CI. This is so we can use the new obs.isCI() function to determine if we're running in a CI env (no GPU) very quickly from the tests.
  • Adds a test for OBS_settings_getVideoDevices.
  • test_osn_advanced_replayBuffer.ts: still flaky on macOS test runner so
    disable on CI runner
  • Set CMAKE_OSX_DEPLOYMENT_TARGET to MacOS 12. now matches SLOBS which is set here

Performance:
New implementation (nanoseconds)

[000:00:00:03.691.221.250][66014221][Info] OBS_settings_getInputAudioDevices took 400125
[000:00:00:03.691.641.333][66014221][Info] OBS_settings_getOutputAudioDevices took 288458

Old (nanoseconds)

[000:00:00:03.530.971.333][65989647][Info] OBS_settings_getInputAudioDevices took 721292
[000:00:00:03.531.601.833][65989647][Info] OBS_settings_getOutputAudioDevices took 479084

The new enumDevices implementation is more efficient since it does not create a dummy_source. Also, it avoids triggering coreaudio to create a reconnect_thread for a dummy device (which saves us another 2ms or so in an async thread because mac-coreaudio polls within the reconnect_thread).

Motivation and Context

When I was looking into profiling missing source devices it reminded me that getDevices() triggers coreaudio to create a reconnect_thread which does later get reaped however it did not seem ideal. But it would be nice to avoid this. Currently, desktop queries getAudioDevices() at least around 14 times so it is nice to optimize this to get the app to load up faster and spin up less threads.

How Has This Been Tested?

Locally, I verified the same audio/video devices are output. Originally, commit 9279b8a was reverted due to breaking the old onboarding flow (no webcam detected) but that is addressed in this PR since we will enumerate video devices using the legacy approach.

Types of changes

  • Performance enhancement (non-breaking change which improves efficiency)

Checklist:

  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reintroduces and refines a getDevices optimization to avoid creating dummy sources (notably preventing CoreAudio from spinning up unnecessary reconnect threads), adds a macOS-specific video device enumeration path to avoid plugin initialization requirements, and updates CI/test utilities to better handle Darwin CI constraints.

Changes:

  • Replace dummy-source-based property enumeration with obs_get_source_properties() for device listing, and add warning logs on property lookup failures.
  • Implement manual macOS video device enumeration via AVFoundation and route OBS_settings_getVideoDevices (macOS) through it.
  • Add/adjust OSN tests and CI flags to better detect Darwin CI and validate video device enumeration output shape.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/osn-tests/util/obs_handler.ts Adds isOnDarwinCI() helper for test gating on macOS CI.
tests/osn-tests/util/error_messages.ts Adds new error messages for video device enumeration tests.
tests/osn-tests/src/test_osn_video.ts Adds a test for OBS_settings_getVideoDevices() return shape and (non-CI) non-empty expectation.
tests/osn-tests/src/test_osn_audio.ts Gates default output device expectation behind isOnDarwinCI().
obs-studio-server/source/nodeobs_settings.cpp Updates getDevices() to avoid dummy source creation; macOS video devices now enumerated via new macOS helper.
obs-studio-server/source/nodeobs_settings-osx.mm New AVFoundation-based macOS video device enumeration implementation.
obs-studio-server/source/nodeobs_settings-osx.h Declares macOS video device enumeration helper.
obs-studio-server/CMakeLists.txt Adds new macOS settings enumeration sources to the build.
.github/workflows/main.yml Explicitly sets CI=true for the macOS test job environment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread obs-studio-server/source/nodeobs_settings-osx.mm Outdated
Comment thread obs-studio-server/source/nodeobs_settings-osx.h Outdated
Comment thread obs-studio-server/source/nodeobs_settings.cpp
Comment thread obs-studio-server/source/nodeobs_settings.cpp Outdated
Comment thread tests/osn-tests/util/obs_handler.ts Outdated
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 6, 2026

CLA assistant check
All committers have signed the CLA.

@sandboxcoder sandboxcoder force-pushed the rno/get-properties branch 3 times, most recently from 32817d2 to 8b48df9 Compare May 7, 2026 15:36
@sandboxcoder sandboxcoder changed the title getDevice optimization MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test May 7, 2026
@sandboxcoder sandboxcoder requested a review from Copilot May 7, 2026 17:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread tests/osn-tests/util/obs_handler.ts
Comment thread obs-studio-server/source/nodeobs_settings.cpp
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Comment thread tests/osn-tests/src/test_osn_advanced_replayBuffer.ts
Comment thread tests/osn-tests/src/test_osn_advanced_replayBuffer.ts
Comment thread tests/osn-tests/src/test_osn_audio.ts Outdated
Comment thread tests/osn-tests/src/test_osn_video.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Comment thread tests/osn-tests/src/test_osn_audio.ts Outdated
Comment thread tests/osn-tests/src/test_osn_video.ts Outdated
Comment thread obs-studio-server/source/nodeobs_settings.cpp Outdated
Comment thread obs-studio-server/source/nodeobs_settings-osx.mm Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Comment thread tests/osn-tests/src/test_osn_audio.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Comment thread CMakeLists.txt
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Comment thread obs-studio-server/source/nodeobs_settings-osx.mm Outdated
Comment thread obs-studio-server/source/nodeobs_settings-osx.mm Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Comment thread CMakeLists.txt
Comment on lines 30 to 34
# Comfigure macos architecture and min version
iF(APPLE)
set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "11.0")
set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "10.15")
set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "12.0")
set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "12.0")
if (NOT CMAKE_OSX_ARCHITECTURES)
Copy link
Copy Markdown
Contributor Author

@sandboxcoder sandboxcoder May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Streamlabs Desktop minimum version is actually MacOS 12 (this version in particular is special, my minspec Intel laptop is set to MacOS 12 while the Mac M4 is using latest). I just never updated the deployment target in the xcode cmake file because we never needed to bump it (only reason I was thinking to bump it now is to make it obvious there is no need for fallback code for anything older then macOS 12) but should have bumped this much earlier...

This matches SLOBS Deployment target now.

@sandboxcoder sandboxcoder force-pushed the rno/get-properties branch from 72cd389 to d4537f7 Compare May 8, 2026 19:19
…es()

* rename legacy getDevices() -> getDevicesUsingDummySource(). Adding
more error checking
* set CI flag for MacOS test runner so it can use obs.isCI()
@sandboxcoder sandboxcoder force-pushed the rno/get-properties branch from 6623119 to d56cf32 Compare May 8, 2026 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants